-
Notifications
You must be signed in to change notification settings - Fork 350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MultiArchitecture support #3309
Conversation
update docker file for camel-k operator change makefile to build multiplatform image add platform parameter to buildah task
add buildah go file inside builder package add Architecture field inside publish task and mapping to buildah
platform attribute moved to BuildahTask
add images-arch images-dev-arch bundle-build-arch for manual build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall very nice accomplishment. There are a few things that may need some adjustment. Also, I'd rename everything you've specified as arch
as arm64
instead, in order to understand it is referring to that particular architecture.
use platform attribute if exists docker.io from defaults.go
I prefer arch against arm64 because i think in the future camel k supports other architectures... i don't know if this feature is part of camel-k roadmap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. It would be nice to include some example or documentation so that everybody know how to use the new feature.
Hi @squakez, how i share the documentation? Have you a package inside the repository or other? Thanks in advance |
Examples are in https://github.com/apache/camel-k/tree/main/examples By the way, we've fixed a few things in the ci recently. I think you can rebase with |
Ok Good! I will add documentation and some check before merge pr |
Hi @squakez , how we can proceed with pr? |
I'm not able to follow up with it this week. If nobody else is having a look in the while, I'll check it again during next week. |
@robertonav20 have you managed to sort those problems out? If tests pass and you confirm it's good to go, I will merge. |
@squakez no unfortunately, i just made an analysis to understand the problem... you can see the details in the previous comment. By the way i tested correctly the integration kit image without that param architecture |
I wonder if you have some older build that is reused. Try to do a |
Ok thanks... i will do these steps in some few days, anyway the tests are failed so how we proceed? |
@squakez i tested again with your instructions but i get the same result
I don't know how to help :( But i think, this is an optional feature, so we can proceed with pr and open another task to fix him. Like this
|
I am curious to see what do you have in the Build configuration. Can you please post the result of |
Here you can find the build yaml
|
The problem is here:
The Buildah configuration is missing the platform. Ie, when I run it, I can see:
I still think it does not work for you because CRDs are not correctly applied. I will rerun the failing tests, and, if they are green and you agree we can merge this. Then, you will be able to try again as soon as it is merged via nightly build installation in order to test it clean. |
The test are failed again, can i help with this? For me we can proceed with pr, i will test the feature with the night build |
Can you please rebase this PR against the latest |
I rebased the branch, can you restart the tests? |
This test is suspiciously failing every time:
It seems it ends up always in timeout. I think it is related to the PR. Can you please have a look? |
Ok, is the only error? |
No, there are several. However, I'd focus on this one as it appears on the builder, which is the part changed in the PR. Hopefully, once we fix them, the rest will clear. |
i think which our problem is the generated crd, because the tests starts to fail from that change... i relaunch the generation of crds, if fail again can you start them? maybe i have something wrong on my pc |
Restarted. I have seen in your branch, only one test is failing (which is a flaky one): https://github.com/robertonav20/camel-k/runs/6948033955?check_suite_focus=true If that is the case here as well, I will squash and merge accordingly. 🤞 |
I rebased the branch with last commit, can you restart the tests ? If they complete successfully we can merge |
Done. FYI, the previous test execution passed correctly, so it looks in good shape now. |
Merged. Thanks for the contribution! I think we can have a blog announcing this experimental feature, feel free to add one into camel-website. I guess we can reuse part of the documentation you already wrote, maybe with some snapshot taken from your tests (and some performance measurement). |
Nice! Thanks to you for your guide! |
The pr introduces:
PS. to enable this behavior just use --build-publish-strategy=Buildah inside install command